Skip to content

Conversation

@cameronvoell
Copy link
Contributor

@cameronvoell cameronvoell commented Jan 23, 2026

Add Client.sendSyncRequest(), extend message preparation with noSend, implement Conversations.streamMessageDeletions(), and upgrade native SDKs to libxmtp 1.9.0 across iOS (XMTP 4.9.0) and Android (org.xmtp:android 4.9.0)

Introduce manual device sync via Client.sendSyncRequest(), support deferred publishing with Conversation.prepareMessage(..., noSend), add real-time deletion streaming with Conversations.streamMessageDeletions(), implement enriched message decoding/serialization (DecodedMessageV2) and deletion/leave-request codecs, expose publish/delete/enriched messages APIs, and pass appData through group creation; bump native dependencies to XMTP 4.9.0.

📍Where to Start

Start with the deletion streaming flow at Conversations.streamMessageDeletions in Conversations.ts, then review the platform handlers in XMTPModule.swift and XMTPModule.kt.

Changes since #765 opened

  • Changed DecodedMessageV2.from() factory method to assign the recursively parsed reactions array or an empty array instead of passing through the decoded reactions directly [68233db]
  • Updated DecodedMessageV2.content() and DecodedMessageV2.decodedContent() methods to document FFI limitations with enrichedMessages() for unknown or custom content types and adjusted codec handling [68233db]
  • Added documentation in conversation tests explaining a removed test and the limitation where enrichedMessages() does not apply JSContentCodec.decode() transformations [68233db]
  • Modified ContentJsonV2.toJsonMap in iOS to serialize unknown and custom content types as structured JSON [a789e15]
  • Added assertions and logging to the custom content type test to validate structured JSON in nativeContent [a789e15]
  • Consolidated DecodedMessageUnionV2 import and removed default initialization of deliveryStatus in DecodedMessageV2 [a789e15]
  • Changed MessageWrapper.encodeToObjV2 to conditionally include optional fields in result dictionary [e13af0a]
  • Removed debug information upload functionality including the uploadDebugInformation exported function, the XMTPDebugInformation.uploadDebugInformation instance method, and the associated test case that verified upload key generation [df5dc25]
  • Added updateAppDataPolicy: 'allow' field to customPermissionsPolicySet objects in group permissions tests [2e00272]
  • Updated reactionV2 content parsing in ContentJson wrapper to use Reaction payload type instead of FfiReactionPayload, changed helper functions from getReactionV2Action and getReactionV2Schema to getReactionAction and getReactionSchema, and populated referenceInboxId field from JSON data instead of using hardcoded empty string [cc2c241]
  • Replaced FfiReactionPayload construction with Reaction construction in the reactionV2 branch of ContentJson.fromJsonObj parser [57d55a3]
  • Changed DeleteMessageCodec.contentType.typeId from 'deletedMessage' to 'deleteMessage' [68d2712]
  • Added test verifying native codec content type IDs match expected native SDK values [68d2712]
  • Added delete message content type handling to JSON serialization [7eed895]
  • Modified reaction object parsing in ContentJson wrapper to handle JSON null values for referenceInboxId [6da1e24]
  • Updated package version from 5.1.0 to 5.2.0 [1f6cb9d]

📊 Macroscope summarized 7eed895. 30 files reviewed, 51 issues evaluated, 33 issues filtered, 1 comment posted

🗂️ Filtered Issues

android/src/main/java/expo/modules/xmtpreactnativesdk/XMTPModule.kt — 0 comments posted, 5 evaluated, 4 filtered
  • line 246: The debugEventsEnabled parameter from authOptions is no longer passed to ClientOptions. The value is still parsed from JSON in authParamsFromJson and stored in AuthParamsWrapper, but the removal of this line means debug events configuration will be silently ignored. If ClientOptions still supports this parameter, users setting debugEventsEnabled: true will not get expected debug event behavior. [ Already posted ]
  • line 593: The change from sigRequest.let to sigRequest?.let in ffiRevokeAllOtherInstallationsSignatureText means the function will now silently return null if client.ffiRevokeAllOtherInstallations() returns null. Callers expecting a non-null signature text string may encounter unexpected null values, potentially causing NullPointerException downstream. [ Low confidence ]
  • line 2424: Catching Exception on line 2424 will also catch CancellationException, which can interfere with structured concurrency. When the coroutine is cancelled (e.g., via subscriptions[...].cancel()), the CancellationException will be caught, logged as an error, and not rethrown. This prevents proper cancellation propagation and could cause the coroutine to not terminate cleanly. The catch block should either catch a more specific exception type or rethrow CancellationException. [ Already posted ]
  • line 2426: In the catch block at line 2426, the code calls cancel() on the same job that is currently executing and failing. This is effectively a no-op since the job is already completing exceptionally, and represents dead code that suggests confusion about the coroutine lifecycle. [ Already posted ]
android/src/main/java/expo/modules/xmtpreactnativesdk/wrappers/ContentJson.kt — 0 comments posted, 2 evaluated, 2 filtered
  • line 154: If referenceInboxId key exists in the JSON but has a null value (e.g., "referenceInboxId": null), reaction.get("referenceInboxId") will return JsonNull (not Kotlin null), so the safe call ?.asString won't protect against it—calling asString on JsonNull throws UnsupportedOperationException. Consider using reaction.get("referenceInboxId")?.takeIf { !it.isJsonNull }?.asString ?: "" instead. [ Already posted ]
  • line 176: The fromJsonObject method does not handle "leaveRequest" or "deleteMessage" JSON keys, but toJsonMap() serializes them with those keys. If a caller serializes a LeaveRequest or DeleteMessageRequest via toJsonMap() and then attempts to deserialize it via fromJson(), it will throw "Unknown content type" exception instead of round-tripping correctly. [ Already posted ]
android/src/main/java/expo/modules/xmtpreactnativesdk/wrappers/ContentJsonV2.kt — 0 comments posted, 10 evaluated, 7 filtered
  • line 143: In fromJsonObject at line 143, the reactionV2 branch creates a ContentJson with FfiReactionPayload as content type, but ContentJson (shown in the diff) was updated to use the SDK Reaction type instead. The comment on line 147 in ContentJson explicitly states "ReactionV2Codec encodes Reaction" - using FfiReactionPayload here will cause a type mismatch when the codec tries to encode the content, likely resulting in a ClassCastException or encoding failure. [ Already posted ]
  • line 143: In ContentJsonV2.fromJsonObject, when parsing reactionV2, the code still creates FfiReactionPayload (line 143) and uses getReactionV2Action/getReactionV2Schema (lines 145-146), but the ReactionV2Codec was updated to encode Reaction type instead. The diff comment in ContentJson.kt explicitly states "Use SDK Reaction type (not FfiReactionPayload); ReactionV2Codec encodes Reaction." This inconsistency means messages created via ContentJsonV2.fromJson with reactionV2 content will fail when encoded because the codec expects Reaction but receives FfiReactionPayload. [ Already posted ]
  • line 143: In ContentJsonV2.fromJsonObject(), the reactionV2 handling creates a ContentJson with FfiReactionPayload type at line 143, but the diff shows that ContentJson.kt was updated to use Reaction type instead. The ReactionV2Codec (per the comment in the updated ContentJson.kt) encodes Reaction, not FfiReactionPayload. This type mismatch will cause encoding failures or incorrect behavior when processing reactionV2 content through ContentJsonV2. [ Already posted ]
  • line 143: In fromJsonObject, the reactionV2 branch creates a ContentJson with FfiReactionPayload as the content type (lines 142-151), but according to the changes in ContentJson.kt, ReactionV2Codec now expects the SDK Reaction type instead. This is an incomplete change - ContentJson.kt was updated to use Reaction with getReactionAction/getReactionSchema, but ContentJsonV2.kt still uses the old FfiReactionPayload with getReactionV2Action/getReactionV2Schema. This type mismatch will cause encoding failures when sending reactionV2 messages via ContentJsonV2.fromJsonObject. [ Already posted ]
  • line 145: At line 145-146, getReactionV2Action and getReactionV2Schema are used which return FfiReactionAction and FfiReactionSchema types, but since the code constructs FfiReactionPayload (which requires these FFI types), if this is corrected to use Reaction type instead, these functions should be replaced with getReactionAction and getReactionSchema which return the SDK-compatible types. [ Already posted ]
  • line 145: In ContentJsonV2.fromJsonObject(), the reactionV2 branch uses getReactionV2Action() and getReactionV2Schema() at lines 145-146, but ContentJson.kt was updated to use getReactionAction() and getReactionSchema() instead. These functions return different types (FfiReactionAction/FfiReactionSchema vs SDK ReactionAction/ReactionSchema), causing type incompatibility with the expected Reaction constructor parameters. [ Already posted ]
  • line 321: At line 321, there is a redundant safe cast (content as? DeleteMessageRequest)?.messageId when content is already declared as DeleteMessageRequest? on line 318. This is unnecessary but harmless. [ Low confidence ]
android/src/main/java/expo/modules/xmtpreactnativesdk/wrappers/EnrichedMessageQueryParamsWrapper.kt — 0 comments posted, 4 evaluated, 4 filtered
  • line 32: If paramsJson contains malformed JSON that is not an object (e.g., "hello", 123, or []), the .asJsonObject call on line 32 will throw an IllegalStateException. Consider wrapping this in a try-catch or validating the JSON structure. [ Already posted ]
  • line 36: When a JSON field exists but has a null value (e.g., {"limit": null}), jsonOptions.has("limit") returns true, but calling .asInt, .asLong, or .asString on a JsonNull element will throw an UnsupportedOperationException. This affects all field extractions (lines 36, 43, 50, 57, 72, 79, 86, 93). The code should check !jsonOptions.get("field").isJsonNull before calling the type conversion methods. [ Already posted ]
  • line 64: If excludeSenderInboxIds exists in the JSON but is not an array (e.g., a string or object), getAsJsonArray("excludeSenderInboxIds") on line 64 will throw an IllegalStateException. [ Already posted ]
  • line 65: If the excludeSenderInboxIds array contains null elements in the JSON (e.g., {"excludeSenderInboxIds": ["id1", null]}), the it.asString call on line 65 will throw an UnsupportedOperationException when it encounters the null element. [ Already posted ]
android/src/main/java/expo/modules/xmtpreactnativesdk/wrappers/PermissionPolicySetWrapper.kt — 0 comments posted, 1 evaluated, 1 filtered
  • line 55: In createPermissionPolicySetFromJson, calling jsonObj.get("updateAppDataPolicy").asString will throw a NullPointerException if the JSON input doesn't contain the updateAppDataPolicy field. This breaks backward compatibility with any existing JSON data that was created before this field was added. The get() method returns null for missing keys, and calling .asString on null crashes. [ Already posted ]
ios/Wrappers/CreateGroupParamsWrapper.swift — 0 comments posted, 1 evaluated, 1 filtered
  • line 22: The cast as? Int64 for disappearStartingAtNs and retentionDurationInNs will silently fail even when valid numeric values are present in the JSON. JSONSerialization returns NSNumber for numeric values, which doesn't bridge directly to Int64. This will cause DisappearingMessageSettings to never be created even when the values are provided. Use (jsonOptions["disappearStartingAtNs"] as? NSNumber)?.int64Value instead. [ Already posted ]
ios/Wrappers/EnrichedMessageQueryParamsWrapper.swift — 0 comments posted, 1 evaluated, 1 filtered
  • line 36: The casts as? Int64 for beforeNs, afterNs, insertedAfterNs, and insertedBeforeNs will silently fail and return nil even when valid numeric values are present in the JSON. JSONSerialization returns NSNumber for numeric values, which doesn't bridge directly to Int64. Use (jsonOptions["beforeNs"] as? NSNumber)?.int64Value pattern instead. [ Already posted ]
ios/Wrappers/MessageWrapper.swift — 1 comment posted, 6 evaluated, 4 filtered
  • line 31: The method encodeToObjV2 is marked with throws but never actually throws an error - all internal throwing calls use try? to swallow errors. While not a runtime crash, this is misleading since encodeV2 uses try encodeToObjV2(model) expecting potential errors that will never come from this method. [ Low confidence ]
  • line 33: Using try? with compactMap on line 33-34 silently drops any reactions that fail to encode. If encodeToObjV2(reaction) throws for any reaction, it will be filtered out of the results without any error or warning. This causes silent data loss where the consumer receives an incomplete reactions array and a reactionCount that doesn't match the number of reactions returned. [ Already posted ]
  • line 123: Similar to the action issue, changing from ReactionV2Schema.fromString() to ReactionSchema(rawValue:) may fail silently. ReactionV2Schema.fromString handles "unicode", "shortcode", "custom" with a fallback to .unknown, but ReactionSchema(rawValue:) returns nil for unrecognized values. If the raw value strings differ between the V2 helper and ReactionSchema enum, the schema would be nil. [ Already posted ]
  • line 385: Inconsistent scheme value between ContentTypeRemoteAttachment (line 385: "https://") and ContentTypeMultiRemoteAttachment (line 395: "https"). If downstream consumers expect a consistent format, this mismatch will cause parsing or URL reconstruction failures. [ Already posted ]
src/lib/Conversations.ts — 0 comments posted, 1 evaluated, 1 filtered
  • line 591: The streamMessageDeletions method assigns the new subscription to this.subscriptions[EventTypes.MessageDeletion] without removing any existing subscription at that key. If streamMessageDeletions is called multiple times on the same Conversations instance (e.g., in a component lifecycle or multiple subscribers), the reference to the previous subscription is overwritten and lost. Since cancelStreamMessageDeletions uses this key to find the subscription to remove, the overwritten subscription can never be cleaned up via that method. This results in a permanent memory leak of the event listener and duplicate execution of callbacks for incoming deletion events. [ Already posted ]
src/lib/DecodedMessageV2.ts — 0 comments posted, 7 evaluated, 5 filtered
  • line 51: The DecodedMessageV2.from method incorrectly propagates the parent message's generic ContentType to nested reactions. When a parent message (e.g., TextCodec) is parsed, its reactions are instantiated as DecodedMessageV2<TextCodec> via fromObject<ContentType>. However, at runtime, reaction.content() decodes the payload using the reaction's actual codec (found via this.contentTypeId from the JSON), returning a reaction object. TypeScript expects a string (due to the TextCodec generic), leading to runtime crashes if string methods (like toUpperCase) are called on the returned object. [ Already posted ]
  • line 60: The DecodedMessageV2.from method deserializes JSON directly into the class properties without converting date strings to Date objects. JSON.parse returns strings for date fields (like sentAt and expiresAt), but the class properties are typed as Date. When a consumer attempts to call Date methods (e.g., msg.sentAt.getTime()) on these properties, the application will crash with TypeError: ... is not a function because the runtime value is a string. [ Already posted ]
  • line 149: The DecodedMessageV2 class fails to validate the existence of nativeContent on the parsed JSON object before assigning it. If the native layer returns a partial or malformed JSON object where nativeContent is missing (undefined), the content() method will throw a TypeError (e.g., Cannot read properties of undefined (reading 'encoded')) when accessing this.nativeContent.encoded. [ Already posted ]
  • line 202: The logic in DecodedMessageV2.content() introduces a critical flaw in how decoders are selected for native content. The loop at lines 199-210 iterates over all registered codecs. The condition allowEmptyProperties.some(...) (lines 202-204) checks if nativeContent has any allowed empty property (e.g., 'text') but fails to verify if that property corresponds to the current codec in the loop. [ Already posted ]
  • line 208: In DecodedMessageV2.content(), the logic iterates over all registered codecs in Client.codecRegistry to find a matching NativeContentCodec. However, it accesses codec.contentKey without first verifying that the property exists on the codec object. Client.codecRegistry can contain both JSContentCodec (which does not have contentKey) and NativeContentCodec (which does). Accessing codec.contentKey on a JSContentCodec will return undefined. While this.nativeContent[undefined] is generally safe in JS (returns undefined), if allowEmptyProperties logic is not met, the loop continues. If a JSContentCodec is encountered, the code does not crash immediately at the property check, but if the loop relies on codec being a NativeContentCodec for the decode call later, we must ensure it is one. More critically, the check ('contentKey' in codec ...) is present, but TypeScript runtime behavior for in operator on objects is safe. The issue here is subtle: if this.nativeContent.hasOwnProperty(prop) is true for an empty property (like text), the code executes (codec as NativeContentCodec<...>).decode(this.nativeContent). If the current codec in the loop happens to be a JSContentCodec (which lacks decode(NativeMessageContent) signature or implementation compatible with it, usually expecting EncodedContent), this will crash or throw a type error at runtime because JSContentCodec.decode takes EncodedContent, not NativeMessageContent. The loop iterates Object.values(Client.codecRegistry), so it mixes both types. [ Already posted ]
src/lib/Dm.ts — 0 comments posted, 4 evaluated, 3 filtered
  • line 143: The prepareMessage method accepts SendOptions (which includes shouldPush), but fails to pass this parameter to the underlying native calls. Specifically, in _prepareWithJSCodec (line 171) and the direct XMTP.prepareMessage call (line 159), the shouldPush option is dropped. This means users cannot control push notification behavior for prepared messages (e.g., suppressing push for silent messages), unlike the send method which correctly processes opts.shouldPush. [ Already posted ]
  • line 162: The prepareMessage method accepts opts: SendOptions (which includes shouldPush?: boolean), but the implementation discards the shouldPush property in both execution paths. When calling _prepareWithJSCodec (line 162), shouldPush is not passed, and _prepareWithJSCodec itself does not accept it as an argument. Similarly, in the direct native call path (line 170), XMTP.prepareMessage is called without any push configuration. Because prepareMessage performs an optimistic send by default (when noSend is false), users invoking prepareMessage with shouldPush: false will have this preference ignored, resulting in unintended push notifications if the underlying default is to push. [ Already posted ]
  • line 199: The publishMessage method triggers the sending of a previously prepared message, but because prepareMessage dropped the shouldPush preference during the initial storage phase, publishMessage has no way to retrieve or respect the user's original intent regarding push notifications. This results in the message being published with default push behavior (typically enabled) regardless of the options provided during preparation. [ Already posted ]

@changeset-bot
Copy link

changeset-bot bot commented Jan 23, 2026

⚠️ No Changeset found

Latest commit: 1f6cb9d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@cameronvoell cameronvoell marked this pull request as ready for review January 28, 2026 22:17
@cameronvoell cameronvoell requested a review from a team as a code owner January 28, 2026 22:17
@cameronvoell cameronvoell merged commit b9937b9 into main Jan 30, 2026
7 checks passed
@cameronvoell cameronvoell deleted the cv/libxmtp-1.9.0-updates branch January 30, 2026 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants